Code reviews are powerful means to improve the code quality, establish best practices and to spread knowledge. However, code reviews can come to nothing or harm interpersonal relations when they are done wrong. Hence, it’s important to pay attention to the human aspects of code reviews. Code reviews require a certain mindset and phrasing techniques to be successful. This post provides both the author and the reviewer with a compass for navigating through a constructive, effective and respectful code review.

TL;DR

Workshop: Effective Code Reviews

I offer a workshop on how to establish an effective code review process and constructive code reviews in your team and company. Check out the details.

Code Reviews Guidelines For the Author

For you, as the author (or “developer”, “submitter”), it’s important to have an open and humble mindset about the feedback you will receive.

Be Humble

We are humans after all. And humans make mistakes. That’s normal. So as long as software will be written by humans, it will contain mistakes.

This doesn’t mean that you should code carelessly or stop writing tests. But this mindset will take away the fear of mistakes and create an atmosphere where making mistakes is accepted and admitting them is desired. This, in turn, is important for criticism during a code review to be accepted. Otherwise, you may end up in endless justifications and rejections because mistakes may be seen as something forbidden and have to be kept hidden. This prevents the required openness for feedback.

The key takeaway here is: Be humble. Mind that everybody’s code can be improved. You are not perfect. So you have to accept that you will make mistakes. It doesn’t matter how good you are, you can still learn and improve. Don’t consider yourself as infallible and don’t infer your professionalism and reliability as a software developer from infallibility and flawlessness. Admitting mistakes shows that you are professional, honest and after all a human being.

Moreover, I believe that the behavior of the team manager is crucial here. They are the role model for the whole team and should demonstrate a error culture by admitting mistakes in the public. And they should also call them “mistakes”.

Besides, you should give the word “mistakes” a positive connotation by seeing them as opportunities to learn - and to call them so.

You Are Not Your Code

Repeat after me: You are not your code. Criticism on your code is not a criticism on you as a human. Don’t connect your self-worth with the code you write. You are still a valuable team member even if there are some flaws in your code.

In the end, programming is just a skill. It improves with training - and this improvement never stops.

You Are on the Same Side

“Criticism is almost never personal in a professional software engineering environment — it’s usually just part of the process of making a better product”. Fitzpatrick, Collins-Sussman: Debugging Teams, page 16

When receiving feedback from the reviewer, always keep in mind that you and the reviewer are on the same side: You want to create a great product.

Mind the IKEA Effect

The IKEA effect is a cognitive bias in which consumers place a disproportionately high value on products they partially created. Wikipedia

The effect was demonstrated by the following experiment: Two groups should price the value of IKEA furniture. One group got already assembled furniture, the other group had to assemble them first. The results showed that the second group was willing to pay 63 % more than the first group.

Applied to software development this means: We place more value into code that we have written. It might be harder for us to accept changes or removal of code that we have created. It’s important to be aware of this bias when we receive feedback because we might be influenced by the IKEA effect.

New Perspectives On Your Code

Every developer has a different background, assumptions, knowledge, and experiences; and so does the reviewer of your code. It’s totally natural that they see your code differently than you do. Moreover, they are not so familiar with the domain or concrete functionality that kept you busy the last days. That’s great because this reveals code that was clear for you, but not for the reviewer.

// Reviewer: "When does this happen?" 
if (article.state == State.INACTIVE) { 
}
// Implicit knowledge here!

vs.

// Reviewer: "Ah, the state means out-of-stock"
boolean articleIsOutOfStock = article.state == State.INACTIVE;
if (articleIsOutOfStock) { 
}
// make knowledge explicit by using expressive names

So code reviews reveal the implicit knowledge that is not expressed in the code yet because it appears natural for you. We are avoiding a tunnel vision.

But the best point here is: You don’t have to be angry with yourself as you often simply can’t see those issues. You only have one perspective. So just be thankful and embrace the opportunity to get a different perspective on your code. It’s so valuable.

Exchange of Best Practices and Experiences

You and the reviewer are not only talking about your code - you are exchanging best practices and experiences. Code reviews are a great medium to establish and internalize good coding styles and best practices. And the exchange works in both directions. So consider code reviews as a valuable source of knowledge and an opportunity to learn.

Code Reviews Guidelines For the Reviewer

For you, as the reviewer, it’s important to pay attention to the way you are formulating your feedback. The phrasing is extremely crucial for your feedback to be accepted.

Use I-Messages

Wrong: “You are writing cryptic code."

Right: “It’s hard for me to grasp what’s going on in this code."

Always formulate your feedback from your point of view by expressing your personal thoughts, feelings, and impressions. Why? Because it’s hard for the author to argue against your personal feelings since they are subjective.

In contrast, You-messages sound like an insinuation and an absolute statement. It’s an attack on the author. They will definitely lead to justifications, rejections and a defensive stance. The author will not be thinking about how they can change, but rather how they can argue with you to show you that you are wrong. So the author will be less open for your feedback.

Using I-messages is the most important feedback rule in general and is clearly not limited to code reviews.

Talk About the Code, Not the Coder

Wrong: “You’re requesting the service multiple times, which is inefficient."

Right: “This code is requesting the service multiple times, which is inefficient."

Take out the person in your feedback. Only talk about the code. Criticism on the code is much harder to take personally because you are simply talking about the code, an objective thing, and not the author. Again, this will improve the acceptance (as long as the author understands that they is not their code).

Moreover, this formulation prevents pointless discussions and finger-pointing like: “No, it wasn’t me introducing this request logic. It was Dave who originally introduced this feature!”.

And finally, talking about the code supports the notion of collective code ownership.

Ask Questions

Wrong: “This variable should have the name ‘userId’."

Right: “What do you think about the name ‘userId’ for this variable**?**"

Asking questions feels much less like a criticism; it’s just a question that the author can answer. But it can trigger a though process which can end with accepting the reviewer’s feedback. Or the author can come up with a new, even better solution. In both cases, the acceptance is much higher.

Moreover, by asking questions you can reveal the intention behind a certain design decision. Maybe there is a good reason for this which you haven’t known. If so, you have discovered the intention without any judgement (due to incomplete knowledge) that may upset the author.

Refer to the Author’s Behavior, Not Their Traits

Wrong: “You are sloppy when it comes to writing tests."

Right: “I believe that you should pay more attention to writing tests."

Another general feedback tip is to criticize only the behavior of the author, not their traits. Why? Traits stick to a human and are really hard to change. That’s why that feedback often feels like an attack on the human being itself and will probably lead to resistance. The author will start to argue with you instead of thinking about how to improve the situation. Behavior, however, can be changed more easily. Criticism on the behavior is less likely to be perceived as a personal attack. The author will be more open to your feedback.

However, usually, it’s not required to talk about the author at all (neither their traits or behavior) in a code review. I strongly suggest to use I-messages, talk about the code or ask questions.

Mind the OIR-Rule of Giving Feedback

Another good tool for giving feedback is to structure the feedback into three parts: Observation, Impact and Request.

The OIR-Rule is a great help for phrasing constructive and nonviolent feedback which doesn’t trigger the author’s defensive thread response.

Accept That There Are Different Solutions

Wrong: “I always use fixed timestamps in tests and you should too."

Right: “I would always use fixed timestamps in tests for better reproducibility but in this simple test, using the current timestamp is also ok."

You have to keep in mind that there are always different solutions to a problem. Most likely, you’ll have a favorite solution, but the author’s solution may also be valid. Don’t force your solution on the author if their solution is also fine. Distinguish between common best practices and your personal taste. Mind that your skepticism may just reflect your personal taste and not an objectively wrong code. Make compromises and be pragmatic.

This mindset should prevent you from being uncompromising, pedantic and from annoying the author, which in turn reduces their openness to further feedback and may harm your relationship.

Don’t Jump in Front of Every Train

Don’t be a pedant. Don’t criticize every single line of code. Again, this would annoy the author and reduce their openness to further feedback and harm your relationship. And if your interpersonal relationship is destroyed, you have a much bigger problem than some not perfectly named variables. Instead, choose wisely the battles you are going to fight. Focus on the flaws and code smells that are most important to you.

I really like the metaphor used in Debugging Teams to emphasis this hint:

“Every time a decision is made, it’s like a train coming through town — when you jump in front of the train to stop it you slow the train down and potentially annoy the engineer driving the train. A new train comes by every 15 minutes, and if you jump in front of every train, not only do you spend a lot of your time stopping trains, but eventually one of the engineers driving the train is going to get mad enough to run right over you. So, while it’s OK to jump in front of some trains, pick and choose the ones you want to stop to make sure you’re only stopping the trains that really matter." Fitzpatrick, Collins-Sussman: Debugging Teams, page 72

Praise

Don’t forget to express your appreciation if you have reviewed good code. Praising doesn’t hurt you but will motivate the author and improve your relationship.

But your praise should be specific, concrete and separated from your criticism. Use different sentences and avoid sandwiching:

Wrong: “Most of your code looks good, but the method calc() is too big."

Right: “I really like the class ProductController, Tim. It has a clear single responsibility, is coherent, and contains nicely named methods. Good Job!
Despite this, I spotted the method calc() which is too big for me."

And last but not least: It’s totally fine to say: “Everything is good!”. No code changes is a valid outcome of a code review. Don’t feel forced to find something in the code.

Three Filters For Feedback

April Wensel proposed a great approach in her talk ‘Compassionate (Yet Candid) Code Reviews’ to check your feedback. Before giving feedback, ask yourself:

Those questions aim at many points that have already been covered in the previous sections. However, I like them because they provide a great mental crash barrier during code reviews.

Is it True?

Wrong: “You should use getter and setter. This code is wrong."

Is it true? This statement assumes an absolute truth, which rarely exists. Avoid the words “right”, “wrong” and “should”. Often, you only refer to your opinion. If so, say it:

Right: “In this case, I would recommend using getter and setter, because…"

Again, you can ask questions:

Right: “Did you consider to use getter and setter?"

If it’s a fact, refer to a source (an official or the team’s style guide):

Right: “According to the Java style guide…"

Is it Necessary?

Wrong: “There is a space missing here."

Is it necessary? I believe that there are more important things to talk about in a code review than missing spaces. Nagging tends to annoy the author. This corresponds with the advice to don’t jump in front of every train.

Wrong: “This code sends a chill down my spine, but I see your intention."

Is it necessary? The first part of the sentence has no sense. The reviewer only tries to show how cool they are. The only effect is that the author will feel bad and attacked. So always check your intention: Are you trying to help or boosting your ego?

Wrong: “We should refactor this whole package."

Is it necessary to refactor the whole package in the scope of the current feature and code review? It’s fine to detect those big refactoring needs, but you should work on them separately. Consider to open a ticket or to have a dedicated meeting or chat with the whole team. Choose an appropriate channel.

Right: “Let’s discuss the refactoring in a dedicated meeting."

So the necessary-question has several aspects: To avoid nagging, unnecessary comments, and out-of-scope work.

Is it Kind?

Wrong: “A factory is badly over-engineered here. The trivial solution is to just use the constructor."

Is it necessary? No, and is it kind? Absolutely not! This statement is shaming. It gives the author the feeling of being stupid.

Being kind does not mean that we grab us by the hands, sing “Kumbaya My Lord”, and stop saying something unpleasant. The point is, that being kind is a smart strategy to give feedback that will be accepted. It’s efficient because you don’t trigger someone’s defensive reaction.

The following statement has the same message but without any shaming:

Right: “This factory feels complicated to me. Have you considered to use a constructor instead?"

The Code Review Cheat Sheet

Rules For the Author

For the author, it’s important to have an open and humble mindset about the feedback they will receive.

• Be humble!
  • Mind that everybody’s code can be improved.
  • You are not perfect.
  • Accept that you will make mistakes.
  • No matter how you good you are, you can still learn and improve.
  • Don’t infer your professionalism and reliability as a software developer from infallibility and flawlessness.
• You are not your code
  • Programming is just a skill. It improves with training – and this never stops.
  • Don’t connect your self-worth with the code you are writing.
• Mind that finally, the reviewer wants the same as you: Creating high-quality software. You are on the same side.
• Mind the IKEA effect. You might place a too high value on your own code.
• Consider feedback as a valuable new perspective on your code
  • It reveals your implicit knowledge that is not expressed in the code yet because it appears natural for you.
  • It avoids a tunnel vision.
• Code reviews are a valuable source of best practices and experiences
• Code reviews are a discussion, not a dictation. It’s fine to disagree, but you have to elaborate your reservations politely and be willing to make compromises.

Rules For the Reviewer

For the reviewer, it’s important to pay attention to the way they formulate the feedback. This is extremely crucial for your feedback to be accepted.

• Use I-messages:
  • Right: “It’s hard for me to grasp what’s going on in this code.”
  • Wrong: “You are writing cryptic code.”
• Talk about the code, not the coder.
  • Right: “This code is requesting the service multiple times, which is inefficient.”
  • Wrong: “You’re requesting the service multiple times, which is inefficient.”
• Ask questions instead of making statements.
  • Right: “What do you think about the name ‘userId’ for this variable**?**”
  • Wrong: “This variable should have the name ‘userId’.”
• Always refer to the behavior, not the traits of the author.
  • Right: “I believe that you should pay more attention to writing tests.”
  • Wrong: “You are sloppy when it comes to writing tests.”
• Accept that there are different solutions
  • Right: “I would do it in another way, but your solution is also fine.”
  • Wrong: “I always do it this way and you should too.”
  • Distinguish between common best practices and your personal taste.
  • Mind that your criticism may just reflect your personal taste and not an objectively wrong code.
  • Make compromises and be pragmatic.
• Don’t jump in front of every train
  • Don’t be a pedant. Don’t criticize every single line in the code. This will annoy the author and reduce their openness to further feedback.
  • Focus on the flaws and code smells that are most important to you.
• Respect and trust the author
  • Nobody writes bad code on purpose.
  • The author wrote the code to the best of their knowledge and belief.
• Mind the OIR-Rule of giving feedback
  • Observation - “This method has 100 lines.”
  • Impact - “This makes it hard for me to grasp the essential logic of this method.”
  • Request - “I suggest extracting the low-level-details into subroutines and give them expressive names.”
• Before giving feedback, ask yourself:
  • Is it true? (opinion != truth)
  • Is it necessary? (avoid nagging, unnecessary comments and out-of-scope work)
  • Is it kind? (no shaming)
• Be humble! You are not perfect and you can also improve.
• It’s fine to say: Everything is good!
• Don’t forget to praise.

Workshop: Effective Code Reviews

I offer a workshop on how to establish an effective code review process and constructive code reviews in your team and company. Check out the details.

Further Reading

• I highly recommend the book Debugging Teams by Brian Fitzpatrick and Ben Collins-Sussman. Without exaggeration, this book inspired me.
• April Wensel: Talk ‘Compassionate (Yet Candid) Code Reviews’
• Matt Stine: The Ten Timeless Commandments of Egoless Programming